Skip to content

Conversation

aheejin
Copy link
Member

@aheejin aheejin commented Jun 25, 2024

This updates the list of features in 'generic' and 'bleeding-edge' CPUs in the backend to match

bool WebAssemblyTargetInfo::initFeatureMap(
llvm::StringMap<bool> &Features, DiagnosticsEngine &Diags, StringRef CPU,
const std::vector<std::string> &FeaturesVec) const {
auto addGenericFeatures = [&]() {
Features["multivalue"] = true;
Features["mutable-globals"] = true;
Features["reference-types"] = true;
Features["sign-ext"] = true;
};
auto addBleedingEdgeFeatures = [&]() {
addGenericFeatures();
Features["atomics"] = true;
Features["bulk-memory"] = true;
Features["exception-handling"] = true;
Features["extended-const"] = true;
Features["half-precision"] = true;
Features["multimemory"] = true;
Features["nontrapping-fptoint"] = true;
Features["tail-call"] = true;
setSIMDLevel(Features, RelaxedSIMD, true);
};
if (CPU == "generic") {
addGenericFeatures();
} else if (CPU == "bleeding-edge") {
addBleedingEdgeFeatures();
}
return TargetInfo::initFeatureMap(Features, Diags, CPU, FeaturesVec);
}

This updates existing CodeGen tests in a way that, if a test has separate RUN lines for a reference-types test and a non-reference-types test, I added -mattr=-reference-types to the no-reftype test's RUN command line. I didn't delete existing -mattr=+reference-types lines in reftype tests because having it helps readability.

Also, when tests is not really about reference-types but they have to updated because they happen to contain call_indirect lines because now call_indirect will take __indirect_function_table as an argument, I just added the table argument to the expected output.

target-features-cpus.ll has been updated reflecting the newly added features.

This updates the list of features in 'generic' and 'bleeding-edge' CPUs
in the backend to match
https://github.com/llvm/llvm-project/blob/4e0a0eae58f7a6998866719f7eb970096a2a52e9/clang/lib/Basic/Targets/WebAssembly.cpp#L150-L175.

It is hard to add a backend test for this, given that we don't have a
convenient way of testing the list of features included in each cpu like
the preprocessor test in Clang:
https://github.com/llvm/llvm-project/blob/4e0a0eae58f7a6998866719f7eb970096a2a52e9/clang/test/Preprocessor/wasm-target-features.c#L158-L208
And the current features for 'generic' and 'bleeding-edge' don't have a
separate backend test anyway.
@llvmbot
Copy link
Member

llvmbot commented Jun 25, 2024

@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-webassembly

Author: Heejin Ahn (aheejin)

Changes

This updates the list of features in 'generic' and 'bleeding-edge' CPUs in the backend to match

bool WebAssemblyTargetInfo::initFeatureMap(
llvm::StringMap<bool> &Features, DiagnosticsEngine &Diags, StringRef CPU,
const std::vector<std::string> &FeaturesVec) const {
auto addGenericFeatures = [&]() {
Features["multivalue"] = true;
Features["mutable-globals"] = true;
Features["reference-types"] = true;
Features["sign-ext"] = true;
};
auto addBleedingEdgeFeatures = [&]() {
addGenericFeatures();
Features["atomics"] = true;
Features["bulk-memory"] = true;
Features["exception-handling"] = true;
Features["extended-const"] = true;
Features["half-precision"] = true;
Features["multimemory"] = true;
Features["nontrapping-fptoint"] = true;
Features["tail-call"] = true;
setSIMDLevel(Features, RelaxedSIMD, true);
};
if (CPU == "generic") {
addGenericFeatures();
} else if (CPU == "bleeding-edge") {
addBleedingEdgeFeatures();
}
.

It is hard to add a backend test for this, given that we don't have a convenient way of testing the list of features included in each cpu like the preprocessor test in Clang:

// RUN: %clang -E -dM %s -o - 2>&1 \
// RUN: -target wasm32-unknown-unknown -mcpu=generic \
// RUN: | FileCheck %s -check-prefix=GENERIC-INCLUDE
// RUN: %clang -E -dM %s -o - 2>&1 \
// RUN: -target wasm64-unknown-unknown -mcpu=generic \
// RUN: | FileCheck %s -check-prefix=GENERIC-INCLUDE
//
// GENERIC-INCLUDE-DAG: #define __wasm_multivalue__ 1{{$}}
// GENERIC-INCLUDE-DAG: #define __wasm_mutable_globals__ 1{{$}}
// GENERIC-INCLUDE-DAG: #define __wasm_reference_types__ 1{{$}}
// GENERIC-INCLUDE-DAG: #define __wasm_sign_ext__ 1{{$}}
//
// RUN: %clang -E -dM %s -o - 2>&1 \
// RUN: -target wasm32-unknown-unknown -mcpu=generic \
// RUN: | FileCheck %s -check-prefix=GENERIC
// RUN: %clang -E -dM %s -o - 2>&1 \
// RUN: -target wasm64-unknown-unknown -mcpu=generic \
// RUN: | FileCheck %s -check-prefix=GENERIC
//
// GENERIC-NOT: #define __wasm_atomics__ 1{{$}}
// GENERIC-NOT: #define __wasm_bulk_memory__ 1{{$}}
// GENERIC-NOT: #define __wasm_exception_handling__ 1{{$}}
// GENERIC-NOT: #define __wasm_extended_const__ 1{{$}}
// GENERIC-NOT: #define __wasm_half_precision__ 1{{$}}
// GENERIC-NOT: #define __wasm_multimemory__ 1{{$}}
// GENERIC-NOT: #define __wasm_nontrapping_fptoint__ 1{{$}}
// GENERIC-NOT: #define __wasm_relaxed_simd__ 1{{$}}
// GENERIC-NOT: #define __wasm_simd128__ 1{{$}}
// GENERIC-NOT: #define __wasm_tail_call__ 1{{$}}
// RUN: %clang -E -dM %s -o - 2>&1 \
// RUN: -target wasm32-unknown-unknown -mcpu=bleeding-edge \
// RUN: | FileCheck %s -check-prefix=BLEEDING-EDGE-INCLUDE
// RUN: %clang -E -dM %s -o - 2>&1 \
// RUN: -target wasm64-unknown-unknown -mcpu=bleeding-edge \
// RUN: | FileCheck %s -check-prefix=BLEEDING-EDGE-INCLUDE
//
// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_atomics__ 1{{$}}
// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_bulk_memory__ 1{{$}}
// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_exception_handling__ 1{{$}}
// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_extended_const__ 1{{$}}
// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_half_precision__ 1{{$}}
// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_multimemory__ 1{{$}}
// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_multivalue__ 1{{$}}
// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_mutable_globals__ 1{{$}}
// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_nontrapping_fptoint__ 1{{$}}
// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_reference_types__ 1{{$}}
// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_relaxed_simd__ 1{{$}}
// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_sign_ext__ 1{{$}}
// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_simd128__ 1{{$}}
// BLEEDING-EDGE-INCLUDE-DAG: #define __wasm_tail_call__ 1{{$}}
And the current features for 'generic' and 'bleeding-edge' don't have a separate backend test anyway.


Full diff: https://github.com/llvm/llvm-project/pull/96584.diff

1 Files Affected:

  • (modified) llvm/lib/Target/WebAssembly/WebAssembly.td (+8-4)
diff --git a/llvm/lib/Target/WebAssembly/WebAssembly.td b/llvm/lib/Target/WebAssembly/WebAssembly.td
index 0dd674426e9e8..97618617ff82f 100644
--- a/llvm/lib/Target/WebAssembly/WebAssembly.td
+++ b/llvm/lib/Target/WebAssembly/WebAssembly.td
@@ -110,13 +110,17 @@ def : ProcessorModel<"mvp", NoSchedModel, []>;
 // consideration given to available support in relevant engines and tools, and
 // the importance of the features.
 def : ProcessorModel<"generic", NoSchedModel,
-                      [FeatureSignExt, FeatureMutableGlobals]>;
+                      [FeatureMultivalue, FeatureMutableGlobals,
+                       FeatureReferenceTypes, FeatureSignExt]>;
 
 // Latest and greatest experimental version of WebAssembly. Bugs included!
 def : ProcessorModel<"bleeding-edge", NoSchedModel,
-                      [FeatureSIMD128, FeatureAtomics,
-                       FeatureNontrappingFPToInt, FeatureSignExt,
-                       FeatureMutableGlobals, FeatureBulkMemory,
+                      [FeatureAtomics, FeatureBulkMemory,
+                       FeatureExceptionHandling, FeatureExtendedConst,
+                       FeatureHalfPrecision, FeatureMultiMemory,
+                       FeatureMultivalue, FeatureMutableGlobals,
+                       FeatureNontrappingFPToInt, FeatureRelaxedSIMD,
+                       FeatureReferenceTypes, FeatureSIMD128, FeatureSignExt,
                        FeatureTailCall]>;
 
 //===----------------------------------------------------------------------===//

Copy link
Collaborator

@tlively tlively left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@llvmbot llvmbot added the llvm:mc Machine (object) code label Jun 25, 2024
@aheejin
Copy link
Member Author

aheejin commented Jun 25, 2024

Sorry, apparently there are tests to be updated 😅

3186ad7 updates tests in a way that, if a test has separate RUN lines for a reference-types test and a non-reference-types test, I added -mattr=-reference-types to the no-reftype test's RUN command line. I didn't delete existing -mattr=+reference-types lines in reftype tests because having it helps readability.

Also, when tests is not really about reference-types but they have to updated because they happen to contain call_indirect lines because now call_indirect will take __indirect_function_table as an argument, I just added the table argument to the expected output.

I think https://github.com/llvm/llvm-project/blob/main/llvm/test/CodeGen/WebAssembly/target-features.ll better be updated separately... Will upload a PR for that and land this after that one lands.

@aheejin aheejin marked this pull request as draft June 25, 2024 22:09
@tlively
Copy link
Collaborator

tlively commented Jun 25, 2024

Test changes so far look good 👍

@aheejin aheejin marked this pull request as ready for review June 26, 2024 20:35
@aheejin
Copy link
Member Author

aheejin commented Jul 2, 2024

Will land this; please let me know if you have any other comments.

@aheejin aheejin merged commit fb6e024 into llvm:main Jul 2, 2024
@aheejin aheejin deleted the llc_default branch July 2, 2024 02:12
cpiaseque pushed a commit to cpiaseque/llvm-project that referenced this pull request Jul 3, 2024
This updates the list of features in 'generic' and 'bleeding-edge' CPUs
in the backend to match

https://github.com/llvm/llvm-project/blob/4e0a0eae58f7a6998866719f7eb970096a2a52e9/clang/lib/Basic/Targets/WebAssembly.cpp#L150-L178

This updates existing CodeGen tests in a way that, if a test has
separate RUN lines for a reference-types test and a non-reference-types
test, I added -mattr=-reference-types to the no-reftype test's RUN
command line. I didn't delete existing -mattr=+reference-types lines in
reftype tests because having it helps readability.

Also, when tests is not really about reference-types but they have to
updated because they happen to contain call_indirect lines because now
call_indirect will take __indirect_function_table as an argument, I just
added the table argument to the expected output.

`target-features-cpus.ll` has been updated reflecting the newly added
features.
kbluck pushed a commit to kbluck/llvm-project that referenced this pull request Jul 6, 2024
This updates the list of features in 'generic' and 'bleeding-edge' CPUs
in the backend to match

https://github.com/llvm/llvm-project/blob/4e0a0eae58f7a6998866719f7eb970096a2a52e9/clang/lib/Basic/Targets/WebAssembly.cpp#L150-L178

This updates existing CodeGen tests in a way that, if a test has
separate RUN lines for a reference-types test and a non-reference-types
test, I added -mattr=-reference-types to the no-reftype test's RUN
command line. I didn't delete existing -mattr=+reference-types lines in
reftype tests because having it helps readability.

Also, when tests is not really about reference-types but they have to
updated because they happen to contain call_indirect lines because now
call_indirect will take __indirect_function_table as an argument, I just
added the table argument to the expected output.

`target-features-cpus.ll` has been updated reflecting the newly added
features.
@CryZe
Copy link

CryZe commented Oct 18, 2024

Some feedback: This broke the majority of the JavaScript bundlers (webpack and co.) which are commonly built on top of webassemblyjs which is very unmaintained and does not support reference-types. I'm not sure, but it may be worth reverting.

@dschuff
Copy link
Member

dschuff commented Oct 18, 2024

Can you say more about what the bundlers are doing, and how they are doing it?
I don't want to be breaking people, but also we surely can't freeze LLVM forever just because some other tool is unmaintained. We are actually planning to enable bulk memory and nontrapping-fptoint soon as well.

@dschuff
Copy link
Member

dschuff commented Oct 18, 2024

Interestingly, it looks like the decoder in webassemblyjs already decodes the table index as an LEB.

@dschuff
Copy link
Member

dschuff commented Oct 18, 2024

actually... sorry, let's have this conversation on WebAssembly/tool-conventions#233 rather than forking it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:WebAssembly llvm:mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants